Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix x-logo vendor extension & parse x-codeSamples from files #760

Merged
merged 36 commits into from
Oct 5, 2020
Merged

fix x-logo vendor extension & parse x-codeSamples from files #760

merged 36 commits into from
Oct 5, 2020

Conversation

Nerzal
Copy link
Contributor

@Nerzal Nerzal commented Jul 24, 2020 •
edited
Loading

Describe the PR

  1. the x-logo vendor extension must reside inside the info element. Fixes x-logo is broken #763
  2. I added the possibility to parse x-codeSamples from files similar to the markdown feature. This removes optical waste from the actual code by moving it into separate files.
  3. Fixes x-tagGroups is broken #781

Relation issue
none

Additional context

  1. To fix the x-logo vendor extension I just used the existing vendor extension parsing logic to achieve that.
  2. NewOperation does now also take functional options. This was needed to hand the path parameter to the Operation
  3. This pr should fix even more vendor extensions beside the x-tagGroups is broken #781 issue, cause there is a bug in the underlying spec library, which always called toLower() on the attributes which generates wrong tags in the documentation

@codecov
Copy link

codecov bot commented Jul 24, 2020 •
edited
Loading

Codecov Report

Merging #760 into master will increase coverage by 1.41%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #760      +/-   ##
==========================================
+ Coverage   80.91%   82.33%   +1.41%     
==========================================
  Files           8        8              
  Lines        1588     1608      +20     
==========================================
+ Hits         1285     1324      +39     
+ Misses        187      168      -19     
  Partials      116      116
Impacted Files Coverage Δ
schema.go 84.50% <ø> (ø)
parser.go 78.60% <80.00%> (+2.88%) ⬆️
operation.go 87.10% <80.55%> (-0.55%) ⬇️
gen/gen.go 95.27% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33e4ac3...a4fa218. Read the comment docs.

@Nerzal Nerzal changed the title fix x-logo vendor extension fix x-logo vendor extension & parse x-codeSamples from files Jul 24, 2020
@Nerzal
Copy link
Contributor Author

Nerzal commented Jul 30, 2020

@easonlin404 @sdghchj @pei0804 could one of you please review? :)
We could need this for several projects in my company 👍

@Nerzal
Copy link
Contributor Author

Nerzal commented Aug 4, 2020

build is green now :)

@Nerzal
Copy link
Contributor Author

Nerzal commented Aug 11, 2020

@easonlin404 could u please review?

sdghchj
sdghchj reviewed Aug 12, 2020
View reviewed changes
Copy link
Member

@sdghchj sdghchj left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I haven't see anywhere SetCodeExampleFilesDirectory is called, that is, codeExampleFilesDir in an operation object has not been initialized

operation.go Show resolved Hide resolved
@Nerzal Nerzal requested a review from sdghchj August 17, 2020 11:29
@Nerzal
Copy link
Contributor Author

Nerzal commented Aug 21, 2020

is the build pipeline broken?

@Nerzal
Copy link
Contributor Author

Nerzal commented Aug 24, 2020 •
edited
Loading

This PR does now fix all vendor extensions, which contain an uppercase letter.
And all comments have been fixed.

@easonlin404 @pei0804 @sdghchj could anyone of you please review again?

@WelverinRaven
Copy link

I'd like to use this feature too.

easonlin404
easonlin404 reviewed Aug 29, 2020
View reviewed changes
Copy link
Member

@easonlin404 easonlin404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://travis-ci.org/github/swaggo/swag/jobs/722247122 The CI still builds failed. @Nerzal could you kindly take a look?

@Nerzal
Copy link
Contributor Author

Nerzal commented Aug 29, 2020

done

@Nerzal
Copy link
Contributor Author

Nerzal commented Aug 31, 2020

The reporting of the status seems to be broken in ci, but the build is green ->
https://travis-ci.org/github/swaggo/swag/builds/722323442

@Nerzal
Copy link
Contributor Author

Nerzal commented Sep 8, 2020

@easonlin404 could you please have a look again? :)

@Nerzal
Copy link
Contributor Author

Nerzal commented Sep 17, 2020 •
edited
Loading

So the CI is broken, which is unrelated to that PR.
It feels like noone is actively maintaining this repo anymore :(

@kplachkov
Copy link

why this merge is taking so long?

@kplachkov
Copy link

@Nerzal when you are available, can you resolve the merge conflicts?

@Nerzal
Copy link
Contributor Author

Nerzal commented Oct 4, 2020

@kplachkov done

@Nerzal
Copy link
Contributor Author

Nerzal commented Oct 4, 2020

Well reporting from travis-ci still seems to be broken.

Build is successful:

grafik

@kplachkov
Copy link

okay, if everything is green lets merge this PR @easonlin404

easonlin404
easonlin404 approved these changes Oct 5, 2020
View reviewed changes
Copy link
Member

@easonlin404 easonlin404 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@easonlin404 easonlin404 merged commit 01fb318 into swaggo:master Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@easonlin404 easonlin404 easonlin404 approved these changes

@sdghchj sdghchj Awaiting requested review from sdghchj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

x-tagGroups is broken x-logo is broken
5 participants
@Nerzal @WelverinRaven @kplachkov @easonlin404 @sdghchj